[msbuild] Validate extracted resource paths in UnpackLibraryResources#25911
[msbuild] Validate extracted resource paths in UnpackLibraryResources#25911rolfbjarne wants to merge 6 commits into
Conversation
Ensure that the resolved path for extracted resources stays within the intended output directory. If an embedded resource name contains path traversal sequences (e.g. '..'), the task now logs an error and skips that resource. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens the UnpackLibraryResources MSBuild task against path traversal by validating that embedded resources can only be extracted within the intended intermediate output subdirectory.
Changes:
- Added a containment check for computed extraction paths in
UnpackLibraryResourcesand logs an error when the resource would extract out-of-bounds. - Added a new localized error string (
E7183) describing the out-of-bounds extraction attempt.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| msbuild/Xamarin.MacDev.Tasks/Tasks/UnpackLibraryResources.cs | Adds extraction path containment validation and error logging before writing embedded resources to disk. |
| msbuild/Xamarin.Localization.MSBuild/MSBStrings.resx | Introduces E7183 for the new extraction path validation error message. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Switch from manual GetFullPath+StartsWith check to the existing PathUtils.IsPathContained helper which also handles symlinks. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Add tests verifying that: - Resources with path traversal sequences are rejected with E7183. - Valid resources are still extracted correctly. Also update E7183 to include the computed path and target directory in the error message for easier diagnosis. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
| using System.Reflection.Metadata; | ||
| using System.Reflection.Metadata.Ecma335; | ||
| using System.Reflection.PortableExecutable; |
| // Add the embedded resource | ||
| var resourceBlob = metadataBuilder.GetOrAddBlob (content); | ||
| metadataBuilder.AddManifestResource ( | ||
| ManifestResourceAttributes.Public, |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Use pre-compiled test assemblies instead of System.Reflection.Metadata APIs to avoid type conflicts between the framework and package versions of System.Reflection.Metadata referenced by the test project. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ests These were compiled with: csc -target:library -out:Traversal.dll -resource:r.txt,__monotouch_content_.._sEvil.txt empty.cs csc -target:library -out:Valid.dll -resource:r.txt,__monotouch_content_sub_sfile.txt empty.cs Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #609bcf5] Build passed (Build macOS tests) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
Replace pre-compiled binary test DLLs with runtime assembly creation using Mono.Cecil. This avoids checking in binary files and makes the test self-contained. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
🔥 [CI Build #8597525] Test results 🔥Test results❌ Tests failed on VSTS: test results 30 tests crashed, 0 tests failed, 0 tests passed. Failures❌ assembly-processing tests🔥 Failed catastrophically on VSTS: test results - assembly-processing (no summary found). Html Report (VSDrops) Download ❌ cecil tests🔥 Failed catastrophically on VSTS: test results - cecil (no summary found). Html Report (VSDrops) Download ❌ dotnettests tests (iOS)🔥 Failed catastrophically on VSTS: test results - dotnettests_ios (no summary found). Html Report (VSDrops) Download ❌ dotnettests tests (MacCatalyst)🔥 Failed catastrophically on VSTS: test results - dotnettests_maccatalyst (no summary found). Html Report (VSDrops) Download ❌ dotnettests tests (macOS)🔥 Failed catastrophically on VSTS: test results - dotnettests_macos (no summary found). Html Report (VSDrops) Download ❌ dotnettests tests (Multiple platforms)🔥 Failed catastrophically on VSTS: test results - dotnettests_multiple (no summary found). Html Report (VSDrops) Download ❌ dotnettests tests (tvOS)🔥 Failed catastrophically on VSTS: test results - dotnettests_tvos (no summary found). Html Report (VSDrops) Download ❌ framework tests🔥 Failed catastrophically on VSTS: test results - framework (no summary found). Html Report (VSDrops) Download ❌ fsharp tests🔥 Failed catastrophically on VSTS: test results - fsharp (no summary found). Html Report (VSDrops) Download ❌ generator tests🔥 Failed catastrophically on VSTS: test results - generator (no summary found). Html Report (VSDrops) Download ❌ interdependent-binding-projects tests🔥 Failed catastrophically on VSTS: test results - interdependent-binding-projects (no summary found). Html Report (VSDrops) Download ❌ introspection tests🔥 Failed catastrophically on VSTS: test results - introspection (no summary found). Html Report (VSDrops) Download ❌ linker tests (iOS)🔥 Failed catastrophically on VSTS: test results - linker_ios (no summary found). Html Report (VSDrops) Download ❌ linker tests (MacCatalyst)🔥 Failed catastrophically on VSTS: test results - linker_maccatalyst (no summary found). Html Report (VSDrops) Download ❌ linker tests (macOS)🔥 Failed catastrophically on VSTS: test results - linker_macos (no summary found). Html Report (VSDrops) Download ❌ linker tests (tvOS)🔥 Failed catastrophically on VSTS: test results - linker_tvos (no summary found). Html Report (VSDrops) Download ❌ monotouch tests (iOS)🔥 Failed catastrophically on VSTS: test results - monotouch_ios (no summary found). Html Report (VSDrops) Download ❌ monotouch tests (MacCatalyst)🔥 Failed catastrophically on VSTS: test results - monotouch_maccatalyst (no summary found). Html Report (VSDrops) Download ❌ monotouch tests (macOS)🔥 Failed catastrophically on VSTS: test results - monotouch_macos (no summary found). Html Report (VSDrops) Download ❌ monotouch tests (tvOS)🔥 Failed catastrophically on VSTS: test results - monotouch_tvos (no summary found). Html Report (VSDrops) Download ❌ msbuild tests🔥 Failed catastrophically on VSTS: test results - msbuild (no summary found). Html Report (VSDrops) Download ❌ sharpie tests🔥 Failed catastrophically on VSTS: test results - sharpie (no summary found). Html Report (VSDrops) Download ❌ windows tests🔥 Failed catastrophically on VSTS: test results - windows (no summary found). Html Report (VSDrops) Download ❌ xcframework tests🔥 Failed catastrophically on VSTS: test results - xcframework (no summary found). Html Report (VSDrops) Download ❌ xtro tests🔥 Failed catastrophically on VSTS: test results - xtro (no summary found). Html Report (VSDrops) Download ❌ Tests on macOS Monterey (12) tests🔥 Failed catastrophically on VSTS: test results - mac_monterey (no summary found). Html Report (VSDrops) Download ❌ Tests on macOS Ventura (13) tests🔥 Failed catastrophically on VSTS: test results - mac_ventura (no summary found). Html Report (VSDrops) Download ❌ Tests on macOS Sonoma (14) tests🔥 Failed catastrophically on VSTS: test results - mac_sonoma (no summary found). Html Report (VSDrops) Download ❌ Tests on macOS Sequoia (15) tests🔥 Failed catastrophically on VSTS: test results - mac_sequoia (no summary found). Html Report (VSDrops) Download ❌ Tests on macOS Tahoe (26) tests🔥 Failed catastrophically on VSTS: test results - mac_tahoe (no summary found). Html Report (VSDrops) Download SuccessesmacOS testsLinux Build VerificationPipeline on Agent |
🔥 [PR Build #8597525] Build failed (Detect API changes) 🔥Build failed for the job 'Detect API changes' (with job status 'Failed') Pipeline on Agent |
🔥 [PR Build #8597525] Build failed (Build packages) 🔥Build failed for the job 'Build packages' (with job status 'Failed') Pipeline on Agent |
Ensure that the resolved path for extracted resources stays within the intended output directory. If an embedded resource name contains path traversal sequences (e.g. '..'), the task now logs an error and skips that resource.